-
Notifications
You must be signed in to change notification settings - Fork 176
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Integration] Modify dirac-admin-get-pilot-output to get remote pilot log #7105
[Integration] Modify dirac-admin-get-pilot-output to get remote pilot log #7105
Conversation
eb6b989
to
7ef7644
Compare
968bc9a
to
5079df4
Compare
@@ -431,7 +431,7 @@ def resetJob(self, jobID): | |||
return result | |||
|
|||
############################################################################# | |||
def getJobPilotOutput(self, jobID, directory=""): | |||
def getJobPilotOutput(self, jobID, directory="", remote=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow it does not feel right that user would have to decide about this (seems too low level to be exposed at API level)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be an other option? If it is used in an Dirac-admin command then it is an expert route. The intention was to have a classic log retrieval mechanism a default one, and the external log retriever only used in special cases, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then things can be tried in this order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could try to get logs in a specific order: classic then remote. The specific order could even be configurable on the server. The point is, that if someone ever wants to test the remote log retrieval and the classic logs come first, then there is no way to get the remote logs by using a dirac-admin
command line if we drop the -r
flag.
So my proposal is:
- remove the
-r
flag - set the default order to classic/remote
- add a config flag to the CS
(remoteLogsPriority=True)
or similar. DefaultFalse.
You would need to rebase and fix the conflicts. |
e4de37d
to
7c8eed5
Compare
I rebased, but there are some strange errors I don't recognise... |
7c8eed5
to
1639db7
Compare
src/DIRAC/WorkloadManagementSystem/Client/PilotLoggingPlugins/FileCacheDownloadPlugin.py
Outdated
Show resolved
Hide resolved
src/DIRAC/WorkloadManagementSystem/Client/PilotLoggingPlugins/FileCacheDownloadPlugin.py
Outdated
Show resolved
Hide resolved
src/DIRAC/WorkloadManagementSystem/Client/PilotLoggingPlugins/FileCacheDownloadPlugin.py
Outdated
Show resolved
Hide resolved
lfn = os.path.join(uploadPath, pilotStamp + ".log") | ||
sLog.info("LFN to download: ", lfn) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really an LFN? Looks like just a path to a file to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is. It is passed to getFile
later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am afraid we have been using this pattern in other places, but os.path
is not the most precise way to manipulate/create LFNs, as this is OS-dependent. We might argue that we only run on Linux so it does not really matter, but more precise would be to at least to use posixpath
instead (@chaen and @atsareg might comment)
src/DIRAC/WorkloadManagementSystem/Client/PilotLoggingPlugins/FileCacheDownloadPlugin.py
Show resolved
Hide resolved
src/DIRAC/WorkloadManagementSystem/Service/PilotManagerHandler.py
Outdated
Show resolved
Hide resolved
src/DIRAC/WorkloadManagementSystem/Service/PilotManagerHandler.py
Outdated
Show resolved
Hide resolved
c026d53
to
705f6b5
Compare
45f05a1
to
6611f60
Compare
6611f60
to
21f3471
Compare
BEGINRELEASENOTES
*WMS
NEW: Enhance dirac-admin-get-pilot-output to download remote pilot logs. Currently only from a SE.
ENDRELEASENOTES
Changes are:
dirac_admin_get_pilot_output.py
- add a -r switch to get remote pilot logs rather than classic ones.DiracAdmin.py
to call a service method for remote logsPilotManagerHandler.py
to load a plugin for pilot logs retrieval.FileCacheDownloadPlugin.py
a plugin to download a log file and return it to a client